Re-fold a day's motion only when that day's gravity moved by ryanbr · Pull Request #2015 · ryanbr/noop

ryanbr · 2026-09-09T03:26:19Z

Problem

The steps calibration fits one coefficient from sixty days of strap motion, and it re-folded all
sixty on every analyzeRecent pass. Each day is a resolveDayOwner plus a gravitySamples
read capped at 200,000 rows, so a worn library pays millions of materialised rows per pass to
re-derive numbers that have not moved.

It also does not scale with the days being re-scored, since it always reads the same sixty
regardless, so a two-day pass can cost more than a twenty-one-day one. Identical on both platforms
(IntelligenceEngine.swift and IntelligenceEngine.kt); Swift hoists the reads off the main actor,
which fixes the jank but not the work, and Android has no hoist at all.

Change

StepsEstimateEngine.dayMotionIntensity is a pure fold over one day's gravity stream. Nothing else
reaches it: no profile field, no baseline, no toggle, no other stream. So a day is re-folded only
when that day's gravity witness moved.

The witness is a new gravityFingerprint / gravityFingerprintWindow: COUNT(*) and
COALESCE(MAX(ts), 0) over gravitySample for one device and window, on the same (deviceId, ts)
index the read walks. A hit replaces a 200,000-row read with a single-row aggregate. Both columns
come from ONE query on both platforms, so an insert cannot land between them and produce a key
describing a state the day was never in.
Deliberately narrower than dayStreamFingerprint, which also counts HR, R-R, respiration,
SpO2, steps, skin temp and sleep state. An ordinary HR offload must not invalidate a motion volume
it cannot have changed. It is not a new signal: this pair is exactly the g segment that
fingerprint already computes for gravity, taken on its own.

Why count and newest-timestamp are enough

dayMotionIntensity sums the distance between CONSECUTIVE samples, so it depends on the set of
x/y/z values and on their order. Order is fixed by ORDER BY ts ASC in both reads. That leaves one
question: can a day's gravity change without moving either column? Only by rewriting a timestamp in
place, and nothing does. Gravity rows are insert-only (StreamStore / the offload path), the
timestamp heal exclusively DELETEs out-of-range rows, and an offload re-inserting an existing ts
is absorbed by the conflict key without changing a value. Insert moves the count, delete moves the
count. There is no in-place ts mutation to reorder a day underneath an unchanged witness.

The cache is in-memory and per-process, beside the day-scan cache it mirrors, and needs no
pass-config signature for the reason above. It never persists and never crosses the .noopbak
boundary; a relaunch pays the full fold once. That is the intent: the cost being closed is the
repeat within a process, where an offload storm fires passes back to back.
A zero fold is cached too. Storing only the days that moved would leave every unworn gap
re-reading its whole stream forever to rediscover that it is empty, which is most of the window on
the sparse libraries this is worst for.

No behaviour change. motionByDay keeps the same m > 0 filter, so the calibration points, the
estimates and #1816's banked-motion flag are built from the same values as before. On Swift a
fingerprint that cannot be read bypasses the cache in both directions rather than keying on
(0, 0), which a genuinely empty day also produces; Kotlin's store surfaces no soft failure there,
so a throw aborts the pass exactly as the gravity read already did.

Why nobody saw it

#1538 added a per-phase cost line, but it brackets the day loop and is emitted the moment that loop
returns. Every phase after it was outside the tally, and the steps calibration runs about 700 lines
past the last number the pass printed. Swift now stamps each post-loop phase and renders them in
execution order beside re-score: done:

analyzeRecent postLoop total=31830ms dayReplay=10ms baselines=12ms score2=1100ms ... steps=28930ms ...

Android measures less, and says so. analyzeRecentOnCpu sits 210 bytes under the JaCoCo
method-size ratchet (55,490 against 55,700) and marking its phases costs about 467. That ratchet's
own note says to extract rather than raise it, and an extraction is a larger change than this one
should carry, so Android brackets the persist helper it can afford. The shared line takes an
explicit scope rather than reporting a postLoop total it never measured. Extending Android to
the rest of the post-loop needs that extraction first, and analyzeRecentOnCpu is untouched by this
PR.

Tests

Mirrored StepsMotionCache and AnalysisPhaseTally suites on both platforms, pinned against the
same literals, covering each input that must invalidate, the empty-day key, the owner boundary,
execution order, the scope, and the clock-skew floor that keeps Swift's round-half-away-from-zero
and Kotlin's round-half-up agreeing.
IntelligenceEngineJacocoBudgetTest is repinned, not loosened: the two new diagnostics take
their own positions in the ordered-operations guard, discriminated by argument expression because
that guard masks string literals.
A new runtime case measures the actual claim rather than asserting it: a second pass over
unchanged days reads the witness for all sixty and the gravity stream for none.
Android analytics + data suites: 2,488 tests, 0 failures. Swift package logic verified against a
compiled standalone harness (no macOS available here); the Swift suites rest on CI.

Cost noted, not hidden

Owner resolution still runs per day. On the default single-device install it short-circuits without
a store read (#970), but a multi-strap install still pays sixty LIMIT-1 probes, exactly as before.
The first pass in a process still folds all sixty days.

The steps phase was identified by @DX23876.

@DX23876

…oved The steps calibration fits one coefficient from sixty days of strap motion, and it re-folded all sixty on EVERY analyzeRecent pass. Each day meant a `resolveDayOwner` plus a gravity read capped at 200,000 rows, so a worn library paid millions of materialised rows per pass to re-derive numbers that had not moved. The phase does not scale with the days being re-scored, since it always reads the same sixty days regardless, which is the property that made it hard to reason about: a two-day pass can cost more than a twenty-one-day one. `StepsEstimateEngine.dayMotionIntensity` is a pure fold over one day's gravity stream. Nothing else reaches it: no profile field, no baseline, no toggle, no other stream. So a day is now re-folded only when its own gravity witness moved. The witness is a COUNT and a MAX over `gravitySample` for that day and device, on the same index the read walks, so a hit replaces a 200,000-row read with a single-row aggregate. Deliberately narrower than the existing `dayStreamFingerprint`, which also counts HR, R-R, respiration, SpO2, steps, skin temp and sleep state: an ordinary HR offload must not invalidate a motion volume it cannot have changed. The cache is in-memory and per-process, exactly like the day-scan cache it sits beside, and needs no config signature for the reason above. It never persists and never crosses the .noopbak boundary; a relaunch pays the full fold once. That is the intent, because the cost being closed is the repeat within a process, where an offload storm fires passes back to back. A zero fold is cached too: storing only the days that moved would leave every unworn gap re-reading its whole stream forever to rediscover that it is empty, which is most of the window on the sparse libraries this is worst for. Behaviour is unchanged. `motionByDay` keeps the same `m > 0` filter, so the calibration points, the estimates and #1816's banked-motion flag are all built from the same values as before. On Swift a fingerprint that cannot be read bypasses the cache in both directions rather than keying on (0, 0), which a genuinely empty day also produces. The second half is why this went unnoticed. #1538 added a per-phase cost line, but it brackets the day loop and is emitted the moment that loop returns, so every phase after it was outside the tally: the steps calibration sat 700 lines past the last number the pass printed. Swift now stamps each post-loop phase and renders them in execution order beside `re-score: done`. Android measures less, and says so. `analyzeRecentOnCpu` sits 210 bytes under the JaCoCo method-size ratchet, and marking its phases costs about 467. That ratchet's own note says to extract rather than raise it, and an extraction is a larger change than this one should carry, so Android brackets the persist helper it can afford and the shared line carries an explicit scope, rather than reporting a `postLoop` total it never measured. Extending it needs that extraction first. The ordered-operations guard in the budget test is repinned rather than loosened: the two new diagnostics are pinned in their own positions, discriminated by argument expression because the guard masks string literals. A new runtime case measures the actual claim, that a second pass over unchanged days reads the witness for all sixty and the gravity stream for none. Mirrored tests both platforms; the Android analytics suite is 2,089 green. Cost noted, not hidden: owner resolution still runs per day. On the default single-device install it short-circuits without a store read, but a multi-strap install still pays sixty LIMIT-1 probes, as it did before. The steps phase was identified by @DX23876.

…re projections live Re-review of the motion cache found the Android witness split across two DAO queries, a COUNT and a MAX. Two reads let an insert land between them and return a count from before it beside a newest-timestamp from after: a key describing a state the day was never in. Harmless in the direction that matters (a torn key misses and re-folds), but it is a witness whose whole job is to describe one moment, and Swift already read both columns in a single aggregate. Both columns now come from one query on Android too, returning a `GravityWitness` projection. That also halves the per-pass round trips, sixty instead of a hundred and twenty, on exactly the path this change exists to make cheaper. The two-query shape was copied from `hrFingerprintWindow`, which has the same torn-read property for the same reason and is worth revisiting separately. `GravityWitness` sits in Entities.kt beside `HrWindowStats`, which is where query-result projections live, rather than at the foot of the DAO. Android analytics + data suites: 2,488 tests green.

…epends on A day is re-folded only when its gravity witness moves, which is sound only while a day's gravity cannot change underneath an unchanged count and newest timestamp. Re-review traced every route that touches `gravitySample` and found the argument holds: inserts conflict-ignore on `(deviceId, ts)`, the timestamp heal only DELETEs out-of-range rows, `reKeyGravity` rewrites `deviceId` and nothing else, and both reads are `ORDER BY ts ASC`. Nothing rewrites a timestamp or a vector in place, so a day cannot be reordered or revalued without moving the count. One link in that chain was load-bearing and unpinned: the conflict strategy. Were a gravity insert ever to REPLACE rather than IGNORE, the vectors under a day would change while its count and newest timestamp held still, and the cache would serve a motion volume for data it no longer describes. Nothing would fail. No read errors, no wrong row count, just a quietly stale number feeding the step estimate. Swift pins it behaviourally in a new `GravityWitnessTests` against a real store: re-inserting a second already banked leaves both the witness and the stored vector alone, an append moves both halves, a backfill moves only the count, and the witness stays scoped to one device and one window. An empty window reports (0, 0) rather than failing, which is the answer the zero-fold caching relies on. Kotlin cannot reach its DAO from a JVM unit test (Room's annotations are not retained at runtime and this module has no Robolectric or in-memory Room), so the twin asserts the declaration in source, the same shape the JaCoCo budget guard already uses for its own invariants. Verified by negative control: flipping the annotation to REPLACE fails the test with the reason attached, so it discriminates rather than merely agreeing. Also renames a test method that read as one word. Android analytics + data suites: 2,489 tests green.

…he read failed Found by auditing the two motion loops against each other. Swift's store reads throw and this block wraps every one in `try?`, so a failed gravity read has always collapsed to an empty list and a zero volume. That cost one pass and corrected itself on the next. Caching it made it cost every pass until that day's gravity happened to change: a transient store error turned into a stale zero feeding the step estimate for the life of the process, with nothing failing to show it. The read now keeps its optionality. A nil is a read that could not be made and is folded fresh without being cached; an empty array is a day that genuinely banked nothing and is cached, which is what stops an unworn gap re-reading its whole stream every pass. Same shape as the witness read directly above it, which already bypassed the cache in both directions when it could not be read. Kotlin has no such branch and does not need one: its repo reads propagate, so a failure aborts the pass before anything is written. That divergence is now stated at the call site, because the obvious "fix" of adding a catch there would swallow the abort and start caching exactly the zeros this change stops trusting. Also checked while comparing: the read cap is 200,000 on both sides (Swift's literal, Kotlin's STREAM_LIMIT), and the witness types differ only as Kotlin's 32-bit Int forces elsewhere, matching `AnalyzeRecentDayCache`. Android analytics + data suites: 2,489 tests green.

Thirty real conflicts. The Android tree is dropped again per FORK_GUIDE; every other hunk was resolved by asking which mechanism this fork actually runs, not by preferring a side. Taken from upstream: • Protocol: ryanbr#2017/ryanbr#2018/ryanbr#2020 — the 5/MG historical optical and raw-IMU layouts, and the v26 window's ABSOLUTE base code beside its deltas. Its migration lands as `v57-ppg-waveform-base-code` (upstream's v44), since this fork's sequence is nine ahead of theirs. • BLE diagnostics ryanbr#1998-ryanbr#2003: a bond end that says WHY, named BLE integers, a session-budgeted hex dump, and "we hold the link" only when the OS says so. • Battery: ryanbr#1951 stop asking a 5/MG for a reading it refuses; ryanbr#1959 poll 0x2A19 for an unbonded one. • Scoring: ryanbr#1978/ryanbr#1982/ryanbr#1990 the RHR/HRV baseline gates, ryanbr#1977 the strain denominator's domain, RMSSD withheld on an over-counted night, ryanbr#1943 the resting-HR bin gate, the closed final HR/HRV window, HR-recovery requiring one CONTINUOUS effort, ryanbr#1984 nights bridged under minSleepMin, ryanbr#2015 motion re-folded only when that day's gravity moved, ryanbr#2016 a provisional Rest. • Charts ryanbr#2004/ryanbr#2005/ryanbr#2007/ryanbr#2009/ryanbr#2011 and ryanbr#1995's hero-ring detail route, ryanbr#1164's "Pending sync" caption under Rest. • Widgets: the heart-rate widget with its trace (ryanbr#1957/ryanbr#1964) and the widget cost counters. • Health: ryanbr#1503 — the Apple Health external UUID drops the strap id, so a re-pair no longer strands every prior record, plus the one-off sweep that reclaims the stranded ones; ryanbr#1205 GPS routes read back from Apple Health. • Storage: rolling retention for ppgWaveformSample, the skin-temp absolute backfill outside the 21-night window, per-table byte estimates. • Units split (body vs exercise distance), per-day wake overrides, the wind-down nudge pinned to its own day, and the German Sie→du conversion (29 entries, merged per LOCALE so the fork's own de edits stayed). Kept on the fork's side, with the reason: • ryanbr#1005's in-memory `dayScanCache` stays out, again: the persisted `DayScanFingerprint` survives relaunch and its key folds the per-day weight and max HR upstream's does not. • The steps-motion cache stays PERSISTED for the same reason. Upstream's failed-read-vs-empty-day distinction IS adopted into it — on a persisted cache a stale zero from a read error would outlive the process that made it. • Every stream read in the day loop keeps `limit: Int.max`. All five are uncapped together; taking StreamReadCap for three of them would leave the loop half-capped, which is worse than either choice. • The coach engine. Upstream's K1-K15 layer is not adopted: its single 40-row `coachMessage` table, its tool-less pre-baked context and its own brief are behind what this fork already ships. Its ADDITIVE half is kept — the on-device voice input, the Siri "Ask Coach" intent, the brief scheduler and brief widget, and streaming for the OpenAI-compatible providers this fork's Anthropic-only StreamingToolClient does not cover. They are wired up in the next commit; `CoachBriefWidget` is deliberately not registered until then. • Liquid's `DomainTheme` labels, value-sampled Charge tint, `LiquidScoreGauge` and `TodayCardSurface`; classic Today's ring order and `LiveEffort.today`. • RootTabView's four-tab shell and `MoreCatalog`: Coach hangs on content here, not on a tab. Two duplicate mechanisms collapsed rather than carried: • `ChargeBreakdownWiring` (upstream) and `ChargeBreakdownFormat` (fork) are the same extraction done twice. The fork's survives, Coupled now reads it, and upstream's TEST is kept — it is coverage this path never had. • Upstream's `CoachMessageStore` is dropped; `CoachConversationStore` already autosaves the whole multi-conversation history. One defect found while resolving, fixed here: `LiveEffort.today` read HR with no explicit limit, so `hrSamples`' 8000-row default truncated it — at ~18k rows a day that covered about ten hours and the live Effort silently stopped climbing. It failed safe (`effectiveEffort` takes the max), which is why it went unseen. `Live5RestFrozenTests` is removed by upstream, which replaces it with `Live5NoGravityRestChainTests`. `local_day_windows_oracle.json` moves out of upstream's android path into `Tests/StrandAnalyticsTests/Oracles/` — NOT `Fixtures/`, which this repo's .gitignore excludes everywhere. Verified: StrandAnalytics 2233, WhoopStore 627, WhoopProtocol 700, StrandImport 293, StrandDesign 120, NoopLocalAccess 14, SemanticMemory 19, OuraProtocol 209 and PolarProtocol 25 all green; macOS and iOS both build; doc_comment_lint clean; `i18n_audit --ci` byte-identical to its pre-merge output (it is red on main for reasons that predate this, and the seven new widget literals were given all nine locales so the gate did not get worse). Six StrandTests were ALREADY failing on main before this merge and still are — `CoachMyLogsAndZonesTests` (the tool census pin was not bumped when the Hevy tools landed) and five in `RawPhysiologyUnionTests`. Every input to them — CoachTools.swift, ToolConsent.swift, Repository.swift and both test files — is byte-identical to main, so this merge neither caused nor fixed them. They went unnoticed because app-build.yml last ran on 2026-08-25. Analysis migration required: YES. The scoring changes above make stored derived values stale; the recipe bump and its resumable migration are the next commit.

The 2026-09-09 sync moves scored VALUES, not just how they are presented, so a `dailyMetric` row computed under v2 no longer means what the current code would produce from the same inputs. Nine changes do it, each on some subset of nights: the RHR and HRV baseline gates (ryanbr#1978/ryanbr#1982/ryanbr#1990), the strain denominator's domain check (ryanbr#1977), RMSSD withheld on a night whose R-R over-counts, the ryanbr#1943 resting-HR bin gate, the closed final HR/HRV window, heart-rate recovery requiring one CONTINUOUS effort, ryanbr#1984's bridged fragmented nights, and ryanbr#2015's gravity-conditional motion re-fold. So: `Analysis migration required: yes`. `currentAnalysisRecipeVersion` 2 → 3. No new migration machinery — the existing one already has the properties CLAUDE.md asks for. `prepareAnalysisRecipe` reads the cursor, sees `.migrate(2, 3)` and runs `runAnalysisMaintenance`, which is a forced `analyzeRecent(maxDays: 21, allowDayReuse: false)` that marks the recipe cursor ONLY after the pass returns, so an interrupted or failed run re-runs rather than silently claiming the rescore. It re-derives; it deletes no raw samples and no user sleep or workout corrections. 21 nights is the narrowest interval provable rather than a round number: every one of the nine is a per-night derivation, and the engine's own scoring window is 21 nights. Older nights keep their v2 values until re-scored for another reason — the same contract v1 → v2 shipped with. Deliberately NOT in this bump: the skin-temp absolute backfill for nights OUTSIDE that window. It is its own resumable walker, fill-only on a NULL column, reached on purpose from the Test Centre rather than on launch, so folding it in would widen the migration to the whole library for a value the rescore does not compute anyway. The decision tests are extended where they were weakest: the three existing cases are written against `current`, so they stay green THROUGH a bump without ever witnessing one. Two literal pins are added — that a v2 install migrates as `2 → 3`, and that an install already at 3 (or rolled back from a 4) never rescores on relaunch, which is the failure mode using a marketing or build number here would cause and which is invisible until someone's phone spends twenty minutes re-scoring after a cosmetic update. The first pin is MEANT to fail on the next bump: that failure is the prompt to answer the migration question for whatever the bump carries. Verified: StrandTests 2950, 0 failures; all nine packages green; macOS and iOS both build; doc_comment_lint clean. Recorded in docs/fork/decisions.md. Analysis migration required: yes (this commit is it).

ryanbr added 4 commits September 9, 2026 15:22

ryanbr merged commit 663e255 into main Sep 9, 2026
16 checks passed

ryanbr deleted the perf/steps-motion-cache branch September 9, 2026 04:17

Navigation Menu

Repository navigation

Re-fold a day's motion only when that day's gravity moved - #2015

Re-fold a day's motion only when that day's gravity moved#2015
ryanbr merged 4 commits into
mainfrom
perf/steps-motion-cache

ryanbr commented Sep 9, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ryanbr commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Why count and newest-timestamp are enough

Why nobody saw it

Tests

Cost noted, not hidden

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ryanbr commented Sep 9, 2026 •

edited

Loading